config.js (1)

199-206: Clarify force semantics and guard against misconfig

As written, NSFS_GLACIER_FORCE_STORAGE_CLASS can rewrite uploads to GLACIER even when NSFS_GLACIER_ENABLED is false, which would later be rejected by NamespaceFS._is_storage_class_supported. Please document that forcing requires NSFS_GLACIER_ENABLED=true, or gate the rewrite accordingly in the header-processing layer.

Would you like me to add a small guard in the header rewrite to check config.NSFS_GLACIER_ENABLED first and log a warning if forcing is set without enabling Glacier?

src/endpoint/s3/s3_utils.js (1)

24-25: Define and export a single source of truth for Glacier-class set

Avoid scattering the set of Glacier-like classes across files. Export a frozen list here and reuse it elsewhere (e.g., namespace_fs).

Apply:

 /** @type {nb.StorageClass} */
 const STORAGE_CLASS_GLACIER_IR = 'GLACIER_IR'; // "S3 Glacier Instant Retrieval"
 /** @type {nb.StorageClass} */
 const STORAGE_CLASS_GLACIER_DA = 'GLACIER_DA'; // "DBS3 specific Storage Class"
+
+// Classes that represent cold/archive semantics in NSFS
+const GLACIER_CLASSES = Object.freeze([
+  STORAGE_CLASS_GLACIER,
+  STORAGE_CLASS_GLACIER_DA,
+  STORAGE_CLASS_GLACIER_IR,
+]);

And simplify parse:

 function parse_storage_class(storage_class) {
   if (!storage_class) return STORAGE_CLASS_STANDARD;
   if (storage_class === STORAGE_CLASS_STANDARD) return STORAGE_CLASS_STANDARD;
-  if (storage_class === STORAGE_CLASS_GLACIER) return STORAGE_CLASS_GLACIER;
-  if (storage_class === STORAGE_CLASS_GLACIER_DA) return STORAGE_CLASS_GLACIER_DA;
-  if (storage_class === STORAGE_CLASS_GLACIER_IR) return STORAGE_CLASS_GLACIER_IR;
+  if (GLACIER_CLASSES.includes(storage_class)) return storage_class;
   throw new Error(`No such s3 storage class ${storage_class}`);
 }

Export it:

 exports.STORAGE_CLASS_GLACIER_IR = STORAGE_CLASS_GLACIER_IR;
 exports.STORAGE_CLASS_GLACIER_DA = STORAGE_CLASS_GLACIER_DA;
+exports.GLACIER_CLASSES = GLACIER_CLASSES;

src/sdk/namespace_fs.js (3)

3613-3623: Support matrix: force vs enable mismatch

Good to include GLACIER_DA and GLACIER_IR in the supported set. One caveat: when NSFS_GLACIER_FORCE_STORAGE_CLASS=true (header rewrite to GLACIER), but NSFS_GLACIER_ENABLED=false, uploads will be rejected here. Either document that forcing implies enabling or allow forcing to bypass this check for GLACIER (and GLACIER_DA) uploads.

I can wire a guard in the header rewrite path or adjust this check; confirm the intended behavior for combinations (3) and (4) in the PR description.

---

3692-3699: Force-expire-on-get: consider GLACIER_DA

If deep-archive should also be expired-on-get, extend the check; if not, please document the difference.

Apply:

-        const storage_class = s3_utils.parse_storage_class(stat.xattr[Glacier.STORAGE_CLASS_XATTR]);
-        if (storage_class !== s3_utils.STORAGE_CLASS_GLACIER) return;
+        const storage_class = s3_utils.parse_storage_class(stat.xattr[Glacier.STORAGE_CLASS_XATTR]);
+        if (![s3_utils.STORAGE_CLASS_GLACIER, s3_utils.STORAGE_CLASS_GLACIER_DA].includes(storage_class)) return;

---

3613-3623: Deduplicate Glacier-class logic by reusing s3_utils

Prefer importing a shared set (e.g., s3_utils.GLACIER_CLASSES) instead of re-declaring glacier_storage_classes locally.

Apply:

-        const glacier_storage_classes = [
-            s3_utils.STORAGE_CLASS_GLACIER,
-            s3_utils.STORAGE_CLASS_GLACIER_DA,
-            s3_utils.STORAGE_CLASS_GLACIER_IR,
-        ];
+        const glacier_storage_classes = s3_utils.GLACIER_CLASSES;

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

src/endpoint/s3/s3_utils.js (2)

383-390: Map invalid storage class to S3Error instead of generic Error

Throwing a generic Error likely surfaces as 500. Return a 400-class S3Error.

-    throw new Error(`No such s3 storage class ${storage_class}`);
+    throw new S3Error({ ...S3Error.InvalidArgument, message: `Unsupported storage class ${storage_class}` });

---

825-833: Scope the override to upload ops to avoid surprising mutations

Limit override to put_object and create MPU; keep behavior centralized and future‑proof.

-function override_storage_class(req) {
-    if (
-        config.NSFS_GLACIER_FORCE_STORAGE_CLASS &&
-        parse_storage_class_header(req) === STORAGE_CLASS_STANDARD
-    ) {
-        req.headers['x-amz-storage-class'] = STORAGE_CLASS_GLACIER;
-    }
-}
+function override_storage_class(req) {
+    if (!config.NSFS_GLACIER_FORCE_STORAGE_CLASS) return;
+    // Only for ops where storage class is meaningful
+    if (!(req.op_name === 'put_object' || req.op_name === 'post_object_uploads')) return;
+    if (parse_storage_class_header(req) === STORAGE_CLASS_STANDARD) {
+        req.headers['x-amz-storage-class'] = STORAGE_CLASS_GLACIER;
+    }
+}

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

src/endpoint/s3/s3_utils.js (1)

23-24: Clarify the comment for STORAGE_CLASS_GLACIER_DA.

The comment "DBS3 specific Storage Class" is unclear. Consider updating it to be more descriptive, similar to the other storage class constants. For example:

 /** @type {nb.StorageClass} */
-const STORAGE_CLASS_GLACIER_DA = 'GLACIER_DA'; // "DBS3 specific Storage Class"
+const STORAGE_CLASS_GLACIER_DA = 'GLACIER_DA'; // "S3 Glacier Deep Archive"

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro